[istream.sentry] Minor tweaks #8198
Conversation
source/iostreams.tex
Outdated
| is | ||
| \tcode{true}, | ||
| \tcode{\exposid{ok_} != false} | ||
| \tcode{\exposid{ok_} != false}; |
There was a problem hiding this comment.
| \tcode{\exposid{ok_} != false}; | |
| \tcode{\exposid{ok_}} is \tcode{false}; |
The == false below should also be changed for local consistency; then we have local consistency about using "is true"/"is false" in this \itemdescr. Not fixing this would be silly if we're already touching this line.
There was a problem hiding this comment.
The original wording is awfully terse. Can we spell this out a bit more, like:
"""
If, after any preparation is completed, is.good() is true, then ok_ is true, otherwise ok_ is false.
"""
Or could we deunpack this even further and say something like "the value of ok_ is is.good()."?
There was a problem hiding this comment.
Well we definitely can't change "ok_ != false" to "ok_ is false"!
I agree with tkoeppe's suggestion, including adding "then" after the third comma.
We could also just say:
After any preparation is completed,
ok_is set to the value ofis.good().
Just saying ok_ = is.good() seems much simpler than describing the following logic, which would never pass code review:
if (is.good() == true)
ok_ = !false;
else
ok_ = false;
There was a problem hiding this comment.
Anything would be better than what's in the draft today ;)
There was a problem hiding this comment.
Also, this sentence should probably be part of the effects instead of remarks?
source/iostreams.tex
Outdated
| whitespace or not. | ||
|
|
||
| \pnum | ||
| To decide if the character \tcode{c} is a whitespace character, |
There was a problem hiding this comment.
While we are here, we can also drop the quote of the constructor signature in the previous paragraph.
Like...it's a Remarks for this constructor. Do we really need to say yet again that we are talking about this constructor?
source/iostreams.tex
Outdated
| This constructor | ||
| uses the currently imbued locale in \tcode{is}, | ||
| to determine whether the next input character is | ||
| whitespace or not. | ||
|
|
||
| \pnum | ||
| To decide if the character \tcode{c} is a whitespace character, | ||
| the constructor performs as if it executes the following code fragment: | ||
| \begin{codeblock} | ||
| const ctype<charT>& ctype = use_facet<ctype<charT>>(is.getloc()); | ||
| if (ctype.is(ctype.space, c) != 0) | ||
| const ctype<charT>& ct = use_facet<ctype<charT>>(is.getloc()); | ||
| if (ct.is(ct.space, c)) | ||
| // \tcode{c} is a whitespace character. | ||
| \end{codeblock} |
There was a problem hiding this comment.
I think these are good improvements so far.
But these two paragraphs are saying the same thing, so I think it would be even more clear (and shorter) if we combined them:
| The currently imbued locale in \tcode{is} is used | |
| to determine whether or not the next input character is whitespace. | |
| The character \tcode{c} is a whitespace character if | |
| \tcode{ct.is(ct.space, c)} is \tcode{true}, | |
| where \tcode{ct} is \tcode{use_facet<ctype<charT>>(is.getloc())}. |
There was a problem hiding this comment.
Thanks @cpplearner and @jwakely for working on this -- this wording can definitely do with improvements, this is great!
source/iostreams.tex
Outdated
| \tcode{setstate(failbit | eofbit)} | ||
| (which may throw | ||
| \tcode{ios_base::failure}). | ||
| After any preparation is completed, \exposid{ok_} is set to the value of \tcode{is.good()}. |
There was a problem hiding this comment.
As this describes the final step, I think it would make more sense to be right at the end, after the "During preparation" sentence (maybe as a separate paragraph).
That way the structure would be:
- The function extracts and discards each whitespace character.
- If extracting a character returns eof, set
failbit|eofbit. - Whitespace is determined using
ctype. - The constructor may set
failbitand throw. (for ... reasons? I guess the footnote covers that) - After any preparation is completed, set
ok_.
What do you think?
There was a problem hiding this comment.
IMO this makes sense if we drop this \remarks macro.
Line 4591 in 61aaf2e
Clarify whitespace character determination in constructor
jwakely
left a comment
jwakely
left a comment
There was a problem hiding this comment.
I think this is a big improvement, thanks
source/iostreams.tex
Outdated
| // \tcode{c} is a whitespace character. | ||
| \end{codeblock} | ||
| The currently imbued locale in \tcode{is} is used | ||
| to determine whether or not the next input character is whitespace. |
There was a problem hiding this comment.
"Whether" already means "whether or not", so can we drop the "or not"?
On the other hand, "if" does not already mean "only if", so can we say "if and only if" below?
And maybe we can also change the full stop after "is whitespace" to a colon to signal that what follows is the explanation of what whitespace is?
| \end{footnote} | ||
|
|
||
| \pnum | ||
| After any preparation is completed, \exposid{ok_} is set to the value of \tcode{is.good()}. |
5 participants
noskipwsis zero" to "noskipwsisfalse". The latter is more conventional sincenoskipwsis abool.ctypetoct. This prevents it from shadowing the class templatestd::ctype.ct.is(…)(which has typebool) and0.